Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Infra UI] Disable Infrastructure and Metrics alerts in Serverless #167978

Conversation

mykolaharmash
Copy link
Contributor

@mykolaharmash mykolaharmash commented Oct 4, 2023

Closes #164683

Summary

This PR disables the infrastructure, metrics and logs alerts rule in Serverless:

  • Deletes the code responsible for the "Metric Anomaly" rule as it was previously disabled with plans to re-enable it as the previous PR describes but that never happened.
  • Adds feature flags for all three types of alert rules
  • Prevents rules registration in serverless based on the feature flags
  • Adds logic for showing/hiding items in the "Alerts and rules" dropdown
  • Disables custom threshold rule in the Infra UI by default in serverless as the rule needs to first be enabled by default by @elastic/actionable-observability team (context)

Dropdown
CleanShot 2023-10-05 at 15 22 48@2x

Host details
CleanShot 2023-10-05 at 15 23 02@2x

How to test

  • Checkout locally Run in Serveless mode
  • Enable, Infra plugin, custom threshold in Infra, and custom threshold rule in general:
xpack.infra.enabled: true
xpack.infra.featureFlags.customThresholdAlertsEnabled: true
xpack.observability.unsafe.thresholdRule.enabled: true
  • Go to /app/metrics/hosts and make sure there are no "Infrastructure" and "Metrics" items in the "Alerts and rules" dropdown
  • Click on "Manage rules" in the "Alerts and rules" dropdown, then "Create rule" to open the rule selection flyout
  • Make sure there are no rules for "Inventory", "Metrics" or "Logs" threshold
  • Run Kibana in traditional mode
  • Make sure the "Alerts and rules" dropdown looks as usual and you can create "Infrastructure" and "Metrics" alerts

@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • /oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@mykolaharmash mykolaharmash force-pushed the 164683-disable-infra-alerts-serverless branch 3 times, most recently from 03eb501 to 600f3f7 Compare October 5, 2023 13:13
@mykolaharmash mykolaharmash added Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting v8.12.0 labels Oct 5, 2023
@mykolaharmash mykolaharmash marked this pull request as ready for review October 5, 2023 13:24
@mykolaharmash mykolaharmash requested review from a team as code owners October 5, 2023 13:24
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

@mykolaharmash mykolaharmash force-pushed the 164683-disable-infra-alerts-serverless branch from 600f3f7 to 4e29533 Compare October 5, 2023 14:03
@kc13greiner kc13greiner self-requested a review October 5, 2023 16:22
@mykolaharmash mykolaharmash force-pushed the 164683-disable-infra-alerts-serverless branch from 4e29533 to bbeb9a0 Compare October 6, 2023 07:58
@mykolaharmash mykolaharmash requested a review from a team as a code owner October 6, 2023 07:58
@mykolaharmash mykolaharmash force-pushed the 164683-disable-infra-alerts-serverless branch from bbeb9a0 to f11efeb Compare October 6, 2023 09:04
@mykolaharmash
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@kc13greiner kc13greiner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jennypavlova jennypavlova self-requested a review October 9, 2023 08:27
@crespocarlos crespocarlos requested review from crespocarlos and removed request for crespocarlos October 9, 2023 10:20
Copy link
Member

@jennypavlova jennypavlova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job 💯
It works as expected - the only thing we discussed and I am adding here as well for visibility:
The xpack.observability.unsafe.thresholdRule.enabled: true setting is needed when the infra plugin is enabled in order to make this work.

EuiPopover,
EuiHeaderLink,
EuiContextMenu,
import { EuiPopover, EuiHeaderLink, EuiContextMenu } from '@elastic/eui';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: We can merge eui imports:

Suggested change
import { EuiPopover, EuiHeaderLink, EuiContextMenu } from '@elastic/eui';
import {
type EuiContextMenuPanelDescriptor,
type EuiContextMenuPanelItemDescriptor,
EuiPopover,
EuiHeaderLink,
EuiContextMenu,
} from '@elastic/eui';

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch, thank you!

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
infra 1.9MB 1.9MB +604.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
infra 105.9KB 105.8KB -34.0B
observability 101.3KB 101.3KB -34.0B
total -68.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
infra 49 45 -4

References to deprecated APIs

id before after diff
infra 50 49 -1

Total ESLint disabled count

id before after diff
infra 58 54 -4

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@mykolaharmash
Copy link
Contributor Author

Good job 💯
It works as expected - the only thing we discussed and I am adding here as well for visibility:
The xpack.observability.unsafe.thresholdRule.enabled: true setting is needed when the infra plugin is enabled in order to make this work.

Thank you for the review @jennypavlova! About the setting issue, I talked to the actionable observability team and @roshan-elastic, for now we've decided to put the whole "Alerts and rules" dropdown behind another feature flag (issue with more context) and wait until Custom Threshold rule is generally available in serverless. But this does not prevent us from merging this PR as now the custom threshold in Infra is hidden by default.

Copy link
Member

@maryam-saeidi maryam-saeidi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AO changes LGTM

@@ -166,7 +166,6 @@ export enum MetricsExplorerChartType {
export enum InfraRuleType {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why we have this type in observability plugin, we should clean this up. (We should check it at #159340)

Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mykolaharmash mykolaharmash merged commit 3e8058b into elastic:main Oct 12, 2023
jennypavlova added a commit that referenced this pull request Oct 17, 2023
Closes #166749
## Summary

This PR adds serverless functional tests for the infra plugin. The main
goal is to make sure that the app loads all the pages and successfully
navigates between them. As we already have functional tests covering the
whole functionality here we want to cover the differences with
serverless and some basic checks ensuring that base functionality works
correctly.

- Update the test to cover disabling the create rule button added
[here](#167978):
([x-pack/test_serverless/functional/test_suites/observability/infra/hosts_page.ts](87fb991#diff-76bd05d3fed2c44187682e27b4030ddb9cba1a36235321e0db1b371ef6739cc9))

⚠️ The tests for the second AC point related to user roles will go in a[
separate issue ](#168931)

Test server run:
`node scripts/functional_tests_server.js
--config=x-pack/test_serverless/functional/test_suites/observability/config.ts`
 Tests run: 
`node scripts/functional_test_runner
--config=x-pack/test_serverless/functional/test_suites/observability/config.ts
--grep "Observability Infra"`
dej611 pushed a commit to dej611/kibana that referenced this pull request Oct 17, 2023
…lastic#167978)

Closes elastic#164683

## Summary

This PR disables the infrastructure, metrics and logs alerts rule in
Serverless:
- Deletes the code responsible for the "Metric Anomaly" rule as it was
[previously disabled](elastic#93813) with
plans to re-enable it as the previous PR describes but that never
happened.
- Adds feature flags for all three types of alert rules
- Prevents rules registration in serverless based on the feature flags
- Adds logic for showing/hiding items in the "Alerts and rules" dropdown
- Disables custom threshold rule in the Infra UI by default in
serverless as the rule needs to first be enabled by default by
@elastic/actionable-observability team
([context](https://elastic.slack.com/archives/C023GDA0WMP/p1696853751040269))

**Dropdown**
![CleanShot 2023-10-05 at 15 22
48@2x](https://github.com/elastic/kibana/assets/793851/fb7344c6-b5ee-4020-bd69-473dcd6be446)

**Host details**
![CleanShot 2023-10-05 at 15 23
02@2x](https://github.com/elastic/kibana/assets/793851/8164f82b-323c-4a2a-8cdc-c65a6c0f0c63)

### How to test

- Checkout locally Run in Serveless mode
- Enable, Infra plugin, custom threshold in Infra, and custom threshold
rule in general:
```
xpack.infra.enabled: true
xpack.infra.featureFlags.customThresholdAlertsEnabled: true
xpack.observability.unsafe.thresholdRule.enabled: true
```
- Go to `/app/metrics/hosts` and make sure there are no "Infrastructure"
and "Metrics" items in the "Alerts and rules" dropdown
- Click on "Manage rules" in the "Alerts and rules" dropdown, then
"Create rule" to open the rule selection flyout
- Make sure there are no rules for "Inventory", "Metrics" or "Logs"
threshold
- Run Kibana in traditional mode
- Make sure the "Alerts and rules" dropdown looks as usual and you can
create "Infrastructure" and "Metrics" alerts

---------

Co-authored-by: Kibana Machine <[email protected]>
dej611 pushed a commit to dej611/kibana that referenced this pull request Oct 17, 2023
Closes elastic#166749
## Summary

This PR adds serverless functional tests for the infra plugin. The main
goal is to make sure that the app loads all the pages and successfully
navigates between them. As we already have functional tests covering the
whole functionality here we want to cover the differences with
serverless and some basic checks ensuring that base functionality works
correctly.

- Update the test to cover disabling the create rule button added
[here](elastic#167978):
([x-pack/test_serverless/functional/test_suites/observability/infra/hosts_page.ts](elastic@87fb991#diff-76bd05d3fed2c44187682e27b4030ddb9cba1a36235321e0db1b371ef6739cc9))

⚠️ The tests for the second AC point related to user roles will go in a[
separate issue ](elastic#168931)

Test server run:
`node scripts/functional_tests_server.js
--config=x-pack/test_serverless/functional/test_suites/observability/config.ts`
 Tests run: 
`node scripts/functional_test_runner
--config=x-pack/test_serverless/functional/test_suites/observability/config.ts
--grep "Observability Infra"`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v8.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Infra UI] Disable Infra UI inventory rule alert creation in serverless
9 participants